FIX: improve CfRadial roundtrips and export after sweep mapping#350
Merged
kmuehlbauer merged 3 commits intoopenradar:mainfrom Apr 23, 2026
Merged
FIX: improve CfRadial roundtrips and export after sweep mapping#350kmuehlbauer merged 3 commits intoopenradar:mainfrom
kmuehlbauer merged 3 commits intoopenradar:mainfrom
Conversation
syedhamidali
force-pushed
the
cfradial-followups
branch
from
f7f2ddf to
1a0bbcf
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #350 +/- ##
==========================================
+ Coverage 93.85% 93.94% +0.08%
==========================================
Files 28 28
Lines 6166 6191 +25
==========================================
+ Hits 5787 5816 +29
+ Misses 379 375 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
syedhamidali
force-pushed
the
cfradial-followups
branch
from
1a0bbcf to
31f67f6
Compare
kmuehlbauer
reviewed
Mar 30, 2026
Collaborator
kmuehlbauer
left a comment
kmuehlbauer
left a comment
There was a problem hiding this comment.
Thanks @syedhamidali, this seems reasonable to do. One comment here.
Collaborator
|
@syedhamidali We can move on with this, please fix the merge conflicts and then we can get this in 👍 |
kmuehlbauer
approved these changes
Apr 1, 2026
syedhamidali
force-pushed
the
cfradial-followups
branch
from
8e9c737 to
a643229
Compare
kmuehlbauer
approved these changes
Apr 23, 2026
Collaborator
kmuehlbauer
left a comment
kmuehlbauer
left a comment
There was a problem hiding this comment.
Need to fix history.md here. Otherwise LGTM.
syedhamidali
force-pushed
the
cfradial-followups
branch
from
64b0fab to
47d9dcd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
map_over_sweepsor into_cfradial1#254 cfradial1 to cfradial2 failed #322history.mdSummary
This PR improves the CfRadial1/CfRadial2 transform and export paths in a few places where the current behavior was inconsistent or too fragile.
The main fixes are:
to_cfradial2()work on root-only datasets returned byxr.open_dataset(..., engine="cfradial1")by reopening the original source when availablegeoreferencing_correctiongroup during CfRadial transforms even when it is emptymap_over_sweeps()workflows that use dataset-wide operations likewhere(), which can broadcast sweep metadata into 2D arraysWhat changed
xradar/transform/cfradial.pyto_cfradial2()now detects the normalized root-only dataset returned byengine="cfradial1"encoding["source"]is available, it reopens the original CfRadial1 file throughopen_cfradial1_datatree(...)georeferencing_correctionis now preserved as a structural group even when it contains no variablesxradar/io/export/cfradial1.pysweep_number,sweep_fixed_angle, andsweep_modeback to scalar form before CfRadial1 exportmap_over_sweeps()operations that applywhere()and broadcast metadata across(time/azimuth, range)tests/transform/test_cfradial.pyengine="cfradial1"datasets passed intoto_cfradial2()encoding["source"]on those datasetsmap_over_sweeps(... where(...))docs/history.mdMotivation
This addresses two related issues:
to_cfradial2()behaved inconsistently depending on how the source dataset was openedto_cfradial1()/io.to_cfradial1()could fail aftermap_over_sweeps()even though the transformed tree looked structurally validIn particular, the exporter previously assumed sweep metadata stayed scalar, which is not true after some dataset-wide masking operations.